Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the undo command #701

Merged
merged 7 commits into from
May 14, 2024
Merged

Implement the undo command #701

merged 7 commits into from
May 14, 2024

Conversation

ima1zumi
Copy link
Member

@ima1zumi ima1zumi commented May 9, 2024

Overview

Implemented the undo method to enable undoing actions on a per-action basis.

Implementation Approach

Executing undo allows reverting to the previous action. The behavior slightly differs from GNU Readline's undo. While GNU Readline groups character insertions within a certain timeframe, I found this to be unintuitive. Inspired by Zsh Line Editor, I made it so that continuous character inputs can be undone one character at a time.

To decide whether to save an operation history, it was necessary to know whether the action was an insertion or a deletion, as the conditions for saving differ between the two. Hence, I added DELETE_COMMANDS. Additionally, I refactored so that keys with any method_symbol would pass through wrap_method_call.

@ima1zumi ima1zumi marked this pull request as ready for review May 10, 2024 18:05
@@ -251,6 +275,8 @@ def reset_variables(prompt = '', encoding:)
@resized = false
@cache = {}
@rendered_screen = RenderedScreen.new(base_y: 0, lines: [], cursor_y: 0)
@past_lines = [["", 0, 0]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe [[[""], 0, 0]].
IRB crash when first key input is undo.

lib/reline.rb Outdated Show resolved Hide resolved
lib/reline/line_editor.rb Outdated Show resolved Hide resolved
@ima1zumi ima1zumi added the enhancement New feature or request label May 13, 2024
def save_past_lines
if @old_buffer_of_lines != @buffer_of_lines
if !@past_lines.empty? && @undoing
@past_lines.pop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about moving this step to def undo?
The current implementation looks like undo logic (== revert buffer_of_lines and revert past_lines) is separated into two parts.

In def input_key, this method can be used like this:

save_past_lines unless @undoing # or maybe rename it to push_past_lines
@undoing = false

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I've fixed it.

Copy link
Member

@tompng tompng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 It works great!

@tompng tompng merged commit 4ab72f9 into ruby:master May 14, 2024
40 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request May 14, 2024
(ruby/reline#701)

* Refactor send

* Implement the undo command

* Fix @past_lines initialization

* Improve assertion

* Hide to save buffer in insert_pasted_text

* Replace @using_delete_command with @undoing

* Refactor `@past_lines`

ruby/reline@4ab72f9cbd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants